Skip to content

Keystone: Implement RegisteredLimit Controller - #782

Merged
mandre merged 10 commits into
k-orc:mainfrom
gndrmnn:implement_registeredlimit
Sep 3, 2026
Merged

Keystone: Implement RegisteredLimit Controller#782
mandre merged 10 commits into
k-orc:mainfrom
gndrmnn:implement_registeredlimit

Conversation

@gndrmnn

@gndrmnn gndrmnn commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

WIP

  • No support for RegionID as k-orc does not have that resource currently
  • RegisteredLimits are unnamed, but have a unique constraint over ResourceName, RegionID, and DefaultValue leading to some interesting challengens. E.g. Declaring two yaml files with two RegisteredLimits with the same RegionID and ResourceName will race to change the same entry.

@github-actions

github-actions Bot commented May 11, 2026

Copy link
Copy Markdown

Failed to assess the semver bump. See logs for details.

@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from 22a08f7 to e46ac56 Compare May 12, 2026 14:31
@github-actions github-actions Bot added the semver:major Breaking change label May 12, 2026
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from e46ac56 to 4964e1d Compare May 13, 2026 13:40
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 4 times, most recently from 8379469 to 027f4aa Compare June 1, 2026 14:23
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 4 times, most recently from b6de206 to e46d4a0 Compare June 29, 2026 14:10
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 11 times, most recently from 4c2f1fe to 8c90198 Compare July 14, 2026 12:02
@gndrmnn

gndrmnn commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@mandre

Hi, this PR is more or less good to be reviewed. I am currently running into a strange problem though: In the registeredlimit-import test we are creating a mandatory Service to use in the import filter. In some cases, the CI pipeline will fail with Waiting for service/<service name> to be ready. Other times, it successfully proceeds to Waiting for OpenStack resource to be created externally. As you can see right now, two CI tests failed while the epoxy one passed.

I have already setup a local keystone instance to test this against, and I can not reproduce the Waiting for service/<service name> to be ready problem. So it looks like this is some kind of timing problem in the test maybe?

What makes this especially weird is, I have looked at the Endpoint actuator and used it as inspiration for this PR. The Endpoint also requires a mandatory ServiceRef and even declares it the same way as I do in the endpoint-import CI test. That test however does not flap at all. So I seem to be doing something wrong, I can not put my finger on right now.

@gndrmnn
gndrmnn marked this pull request as ready for review July 15, 2026 07:02
@mandre

mandre commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

@mandre

Hi, this PR is more or less good to be reviewed. I am currently running into a strange problem though: In the registeredlimit-import test we are creating a mandatory Service to use in the import filter. In some cases, the CI pipeline will fail with Waiting for service/<service name> to be ready. Other times, it successfully proceeds to Waiting for OpenStack resource to be created externally. As you can see right now, two CI tests failed while the epoxy one passed.

Hi @gndrmnn, I won't be able to look at this right away. Perhaps @eshulman2, @dlaw4608, or @winiciusallan can do a first round of reviews, and might have ideas on what could be causing the tests to fail. I'll look at this when I'm back.

winiciusallan
winiciusallan reviewed Jul 17, 2026
View reviewed changes

@winiciusallan winiciusallan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gndrmnn, try the suggestion and let's see how the tests behave. In the meantime, I'll review the rest of your code. Thanks for working on this.

Comment thread internal/controllers/registeredlimit/controller.go
@gndrmnn
gndrmnn marked this pull request as draft July 20, 2026 07:09
@chenwng chenwng mentioned this pull request Jul 20, 2026
Open
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from 8c90198 to 85d8901 Compare July 20, 2026 09:58
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 3 times, most recently from 6147781 to d5b3186 Compare July 22, 2026 07:54
@gndrmnn gndrmnn mentioned this pull request Jul 22, 2026
Open
mandre
mandre reviewed Aug 12, 2026
View reviewed changes

@mandre mandre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this looks very good. Just a few cleanup needed before we can merge it.

Comment thread api/v1alpha1/registeredlimit_types.go
Comment thread api/v1alpha1/registeredlimit_types.go
Comment thread internal/controllers/registeredlimit/actuator.go Outdated
// TODO(scaffolding): Add more fields
ServiceID: serviceID,
ResourceName: resource.ResourceName,
DefaultLimit: int(*resource.DefaultLimit),

@mandre mandre Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only seems to be affecting creation, because the field in UpdateOpts doesn't have the required:true marker.

So let's return a terminal error here when DefaultLimit is 0, pointing to the gophercloud bug.

Comment thread internal/osclients/registeredlimit.go
Comment thread api/v1alpha1/registeredlimit_types.go Outdated
@gndrmnn
Scaffolding for the RegisteredLimit controller
56005f4 
$ go run ./cmd/scaffold-controller -interactive=false \
    -kind=RegisteredLimit \
    -gophercloud-client=NewIdentityV3 \
    -gophercloud-module=github.com/gophercloud/gophercloud/v2/openstack/identity/v3/registeredlimits \
    -gophercloud-type=RegisteredLimit \
    -openstack-json-object=registered_limits \
    -required-create-dependency=Service \
    -import-dependency=Service

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
94accd9 
Register with the resource generator

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
bebd8e4 
Add the OpenStack client to scope

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
0f9275c 
Register the controller

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
gndrmnn marked this pull request as draft September 1, 2026 13:31
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from d5b3186 to ce4d15b Compare September 1, 2026 14:33
@gndrmnn
RegisteredLimit Adapt to BatchCreate API
8cf520a 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch 3 times, most recently from 24b5ade to 5f98b9c Compare September 2, 2026 14:08
@gndrmnn
gndrmnn marked this pull request as ready for review September 2, 2026 14:55
@gndrmnn
gndrmnn requested a review from mandre September 2, 2026 14:55
mandre
mandre reviewed Sep 2, 2026
View reviewed changes
Comment thread api/v1alpha1/registeredlimit_types.go Outdated
Comment thread internal/controllers/registeredlimit/actuator.go Outdated
Comment thread internal/controllers/registeredlimit/actuator.go Outdated
Comment thread internal/controllers/registeredlimit/actuator.go Outdated
Comment thread internal/controllers/registeredlimit/actuator.go
Comment thread internal/controllers/registeredlimit/tests/registeredlimit-update/01-updated-resource.yaml Outdated
@gndrmnn
RegisteredLimit Type Declaration
fe07986 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
RegisteredLimit Actuator and Status
2757d01 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from 5f98b9c to 3188867 Compare September 3, 2026 09:28
@gndrmnn
RegisteredLimit Tests and Config Samples
b7cc943 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Scaffolding for the RegisteredLimit controller
5921193 
Generate the OLM bundle

On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
Add RegisteredLimits to README
d63f4c2 
On-behalf-of: SAP nils.gondermann@sap.com
@gndrmnn
gndrmnn force-pushed the implement_registeredlimit branch from 3188867 to d63f4c2 Compare September 3, 2026 09:57
@gndrmnn
gndrmnn requested a review from mandre September 3, 2026 10:34
mandre
mandre approved these changes Sep 3, 2026
View reviewed changes

@mandre mandre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@mandre
mandre added this pull request to the merge queue Sep 3, 2026
Merged via the queue into k-orc:main with commit 3f02592 Sep 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dlaw4608 dlaw4608 dlaw4608 left review comments
@winiciusallan winiciusallan winiciusallan left review comments
@mandre mandre mandre approved these changes

Assignees

No one assigned

Labels

semver:major Breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gndrmnn @mandre @dlaw4608 @winiciusallan